sysroot: Rework how we find booted deployment

I was looking at this code in prep for "staging" deployments,
and there are several cleanups to be made here.  The first
thing I noticed is that we look for the `ostree=` kernel argument,
but the presence of that should be exactly equivalent to having
`/run/ostree-booted` exist.  We just added a member variable for
that, so let's make use of it.

Related to this, we were erroring out if we had the karg but
didn't find a deployment.  But this can happen if e.g. one is
using `ostree admin --sysroot` from an ostree-booted system!  It's
actually a bit surprising no one has reported this so far; I guess
in the end people are either using non-ostree systems or running
from containers.

Let's add a member variable `root_is_sysroot` that we can use
to determine if we're looking at `/`.  Then, our more precise
"should find a booted deployment" state is when both `ostree_booted`
and `root_is_sysroot` are TRUE.

Next, rather than walking all of the deployments after parsing,
we can inline the `fstatat()` while parsing.  The mild ugly
thing about this is assigning to the sysroot member variable while
parsing, but I will likely clean that up later, just wanted to avoid
rewriting everything in one go.

Closes: #1497
Approved by: jlebon

diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h
index cb4aac9d97e109ee5a68504ebb8c29301d1ec409..b2776e8da1a2411771ecc12fc650e83a4ba1436a 100644 (file)
--- a/src/libostree/ostree-sysroot-private.h
+++ b/src/libostree/ostree-sysroot-private.h
@@ -51,6 +51,10 @@ struct OstreeSysroot {
 
   gboolean loaded;
   gboolean ostree_booted;
+  gboolean root_is_sysroot; /* TRUE if sysroot_fd is pointed to rootfs "/" */
+  /* The device/inode for /, used to detect booted deployment */
+  dev_t root_device;
+  ino_t root_inode;
 
   gboolean is_physical; /* TRUE if we're pointed at physical storage root and not a deployment */
   GPtrArray *deployments;

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index 3294e83d77c8eb2c3cd264e26ac2cf8e939bece3..1cf7779ae04832b3593097c9112af1e3561ee150 100644 (file)
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -37,13 +37,6 @@
 #include "ostree-bootloader-syslinux.h"
 #include "ostree-bootloader-grub2.h"
 
-static gboolean
-find_booted_deployment (OstreeSysroot       *self,
-                        GPtrArray           *deployments,
-                        OstreeDeployment   **out_deployment,
-                        GCancellable        *cancellable,
-                        GError             **error);
-
 /**
  * SECTION:ostree-sysroot
  * @title: Root partition mount point
@@ -642,6 +635,24 @@ parse_deployment (OstreeSysroot       *self,
                        &deployment_dfd, error))
     return FALSE;
 
+  /* See if this is the booted deployment */
+  const gboolean looking_for_booted_deployment =
+    (self->ostree_booted && self->root_is_sysroot &&
+     !self->booted_deployment);
+  gboolean is_booted_deployment = FALSE;
+  if (looking_for_booted_deployment)
+    {
+      struct stat stbuf;
+      if (!glnx_fstat (deployment_dfd, &stbuf, error))
+        return FALSE;
+      /* A bit ugly, we're assigning to a sysroot-owned variable from deep in
+       * this parsing code. But eh, if something fails the sysroot state can't
+       * be relied on anyways.
+       */
+      is_booted_deployment = (stbuf.st_dev == self->root_device &&
+                              stbuf.st_ino == self->root_inode);
+    }
+
   g_autoptr(GKeyFile) origin = NULL;
   if (!parse_origin (self, deployment_dfd, deploy_basename, &origin,
                      cancellable, error))
@@ -672,6 +683,8 @@ parse_deployment (OstreeSysroot       *self,
 
   g_debug ("Deployment %s.%d unlocked=%d", treecsum, deployserial, ret_deployment->unlocked);
 
+  if (is_booted_deployment)
+    self->booted_deployment = g_object_ref (ret_deployment);
   if (out_deployment)
     *out_deployment = g_steal_pointer (&ret_deployment);
   return TRUE;
@@ -797,14 +810,30 @@ ostree_sysroot_load_if_changed (OstreeSysroot  *self,
   if (!ensure_repo (self, error))
     return FALSE;
 
-  /* If we didn't check already, see if we have the global ostree-booted flag;
+  /* Gather some global state; first if we have the global ostree-booted flag;
    * we'll use it to sanity check that we found a booted deployment for example.
+   * Second, we also find out whether sysroot == /.
    */
   if (!self->loaded)
     {
       if (!glnx_fstatat_allow_noent (AT_FDCWD, "/run/ostree-booted", NULL, 0, error))
         return FALSE;
       self->ostree_booted = (errno == 0);
+
+      { struct stat root_stbuf;
+        if (!glnx_fstatat (AT_FDCWD, "/", &root_stbuf, 0, error))
+          return FALSE;
+        self->root_device = root_stbuf.st_dev;
+        self->root_inode = root_stbuf.st_ino;
+      }
+
+      struct stat self_stbuf;
+      if (!glnx_fstat (self->sysroot_fd, &self_stbuf, error))
+        return FALSE;
+
+      self->root_is_sysroot =
+        (self->root_device == self_stbuf.st_dev &&
+         self->root_inode == self_stbuf.st_ino);
     }
 
   int bootversion = 0;
@@ -847,11 +876,19 @@ ostree_sysroot_load_if_changed (OstreeSysroot  *self,
     {
       OstreeBootconfigParser *config = boot_loader_configs->pdata[i];
 
+      /* Note this also sets self->booted_deployment */
       if (!list_deployments_process_one_boot_entry (self, config, deployments,
                                                     cancellable, error))
-        return FALSE;
+        {
+          g_clear_object (&self->booted_deployment);
+          return FALSE;
+        }
     }
 
+  if (self->ostree_booted && self->root_is_sysroot
+      && !self->booted_deployment)
+    return glnx_throw (error, "Unexpected state: /run/ostree-booted found and in / sysroot but not in a booted deployment");
+
   g_ptr_array_sort (deployments, compare_deployments_by_boot_loader_version_reversed);
   for (guint i = 0; i < deployments->len; i++)
     {
@@ -859,13 +896,6 @@ ostree_sysroot_load_if_changed (OstreeSysroot  *self,
       ostree_deployment_set_index (deployment, i);
     }
 
-  if (!find_booted_deployment (self, deployments, &self->booted_deployment,
-                               cancellable, error))
-    return FALSE;
-  /* Sanity check; note the converse case is fine */
-  if (self->booted_deployment && !self->ostree_booted)
-    return glnx_throw (error, "Unexpected state: In a booted deployment but no /run/ostree-booted?");
-
   /* Determine whether we're "physical" or not, the first time we initialize */
   if (!self->loaded)
     {
@@ -1104,84 +1134,6 @@ _ostree_sysroot_join_lines (GPtrArray  *lines)
   return g_string_free (buf, FALSE);
 }
 
-static gboolean
-parse_kernel_commandline (OstreeKernelArgs  **out_args,
-                          GCancellable       *cancellable,
-                          GError            **error)
-{
-  g_autoptr(GFile) proc_cmdline = g_file_new_for_path ("/proc/cmdline");
-  g_autofree char *contents = NULL;
-  gsize len;
-
-  if (!g_file_load_contents (proc_cmdline, cancellable, &contents, &len, NULL,
-                             error))
-    return FALSE;
-
-  g_strchomp (contents);
-  *out_args = _ostree_kernel_args_from_string (contents);
-  return TRUE;
-}
-
-static gboolean
-find_booted_deployment (OstreeSysroot       *self,
-                        GPtrArray           *deployments,
-                        OstreeDeployment   **out_deployment,
-                        GCancellable        *cancellable,
-                        GError             **error)
-{
-  struct stat root_stbuf;
-  struct stat self_stbuf;
-  g_autoptr(OstreeDeployment) ret_deployment = NULL;
-
-  if (stat ("/", &root_stbuf) != 0)
-    return glnx_throw_errno_prefix (error, "stat /");
-
-  if (!ensure_sysroot_fd (self, error))
-    return FALSE;
-
-  if (fstat (self->sysroot_fd, &self_stbuf) != 0)
-    return glnx_throw_errno_prefix (error, "fstat");
-
-  if (root_stbuf.st_dev == self_stbuf.st_dev &&
-      root_stbuf.st_ino == self_stbuf.st_ino)
-    {
-      g_autoptr(OstreeKernelArgs) kernel_args = NULL;
-      if (!parse_kernel_commandline (&kernel_args, cancellable, error))
-        return FALSE;
-
-      const char *bootlink_arg = _ostree_kernel_args_get_last_value (kernel_args, "ostree");
-      if (bootlink_arg)
-        {
-          for (guint i = 0; i < deployments->len; i++)
-            {
-              OstreeDeployment *deployment = deployments->pdata[i];
-              g_autofree char *deployment_path = ostree_sysroot_get_deployment_dirpath (self, deployment);
-              struct stat stbuf;
-
-              if (fstatat (self->sysroot_fd, deployment_path, &stbuf, 0) != 0)
-                return glnx_throw_errno_prefix (error, "fstatat");
-
-              if (stbuf.st_dev == root_stbuf.st_dev &&
-                  stbuf.st_ino == root_stbuf.st_ino)
-                {
-                  ret_deployment = g_object_ref (deployment);
-                  break;
-                }
-            }
-
-          if (ret_deployment == NULL)
-            return glnx_throw (error, "Unexpected state: ostree= kernel argument found, but / is not a deployment root");
-        }
-      else
-        {
-          /* Not an ostree system */
-        }
-    }
-
-  ot_transfer_out_value (out_deployment, &ret_deployment);
-  return TRUE;
-}
-
 /**
  * ostree_sysroot_query_deployments_for:
  * @self: Sysroot